Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Share manifests #740

Merged
merged 12 commits into from
Nov 9, 2023
Merged

Share manifests #740

merged 12 commits into from
Nov 9, 2023

Conversation

visr
Copy link
Member

@visr visr commented Nov 5, 2023

One frequent reason that libribasim and ribasim_cli builds break is because we add a dependency to the core, and we forget to resolve the Manifests of libribasim and ribasim_cli. (e.g. #484).

This adds an entry like manifest = "../Manifest.toml" to the core, libribasim and ribasim_cli Project.toml, which all point to the new root Manifest. With this they still have their own Project, but a shared Manifest. This option exists to make it easier to work with multiple related projects in one monorepo.

As a follow-up we could consider also doing We also do this for the remaining two projects, docs and create_binaries. create_binaries doesn't depend on Ribasim, so there is not as much benefit. docs does, currently not via Project.toml, but via adding it to the LOAD_PATH. It would be cleaner to also use a shared Manifest there. One thing to check if the unmaintained docs dependencies don't hold back Ribasim dependencies, see #621 (EDIT: they don't).

This is on top of #738, but could be separated out if needed.

Also simplifies the docs, and has a not-directly-related commit 8403bef that runs CompatHelper for all projects, not just core.

For reference, the root project is populated like this: dev ./core ./docs build/create_binaries build/libribasim build/ribasim_cli.

visr added 8 commits November 7, 2023 09:17
Running it on the root project is not needed since that doesn't have a compat section.
So we always have everything downloaded at once, not separate subsets depending on the sub-project.
Fixes this error:
```
WARNING: --output requested, but no modules defined during run
└  ┌ Error: Pkg.precompile error
```
@visr visr mentioned this pull request Nov 7, 2023
3 tasks
@visr visr marked this pull request as draft November 7, 2023 10:44
@visr visr mentioned this pull request Nov 7, 2023
2 tasks
@visr
Copy link
Member Author

visr commented Nov 7, 2023

Blocked since this seems to be hitting an issue on Julia 1.9.0 on TeamCity that is fixed in the latest 1.9 release, see #738 (comment).

@visr visr added blocker Someone should do this ASAP and removed blocker Someone should do this ASAP labels Nov 7, 2023
@visr visr marked this pull request as ready for review November 9, 2023 10:13
@Hofer-Julian Hofer-Julian merged commit 21200de into main Nov 9, 2023
17 checks passed
@Hofer-Julian Hofer-Julian deleted the share-manifest branch November 9, 2023 10:55
visr added a commit that referenced this pull request Nov 10, 2023
Follow up to #740.

---------

Co-authored-by: Hofer-Julian <[email protected]>
visr added a commit that referenced this pull request Nov 13, 2023
One frequent reason that `libribasim` and `ribasim_cli` builds break is
because we add a dependency to the core, and we forget to resolve the
Manifests of `libribasim` and `ribasim_cli`. (e.g. #484).

This adds an entry like `manifest = "../Manifest.toml"` to the `core`,
`libribasim` and `ribasim_cli` Project.toml, which all point to the new
`root` Manifest. With this they still have their own Project, but a
shared Manifest. This option exists to make it easier to work with
multiple related projects in one monorepo.

~~As a follow-up we could consider also doing~~ We also do this for the
remaining two projects, `docs` and `create_binaries`. `create_binaries`
doesn't depend on Ribasim, so there is not as much benefit. `docs` does,
currently not via Project.toml, but via adding it to the LOAD_PATH. It
would be cleaner to also use a shared Manifest there. One thing to check
if the unmaintained docs dependencies don't hold back Ribasim
dependencies, see #621 (EDIT: they don't).

~~This is on top of #738, but could be separated out if needed.~~

Also simplifies the docs, and has a not-directly-related commit
8403bef that runs CompatHelper for all
projects, not just core.

For reference, the root project is populated like this: `dev ./core
./docs build/create_binaries build/libribasim build/ribasim_cli`.
visr added a commit that referenced this pull request Nov 13, 2023
Follow up to #740.

---------

Co-authored-by: Hofer-Julian <[email protected]>
@visr visr mentioned this pull request Nov 14, 2023
visr added a commit that referenced this pull request Nov 14, 2023
To address an issue with running the same ribasim binary simultaneously
from different workers.

```
  InitError(mod=:WindowsTimeZoneIDs, error=Base.IOError(msg="unlink("D:\\buildAgent\\work\\c463fd4bc9a9a18\\imod_collector_devel\\ribasim\\share\\julia\\scratchspaces\\f269a46b-ccf7-5d73-abea-4c690281aa53\\build\\local\\windowsZones.xml"): resource busy or locked (EBUSY)", code=-4082)
```

In making this PR I realized I forgot in #740 to actually run
`strip_cldr`.
visr added a commit that referenced this pull request Nov 14, 2023
To address an issue with running the same ribasim binary simultaneously
from different workers.

```
  InitError(mod=:WindowsTimeZoneIDs, error=Base.IOError(msg="unlink("D:\\buildAgent\\work\\c463fd4bc9a9a18\\imod_collector_devel\\ribasim\\share\\julia\\scratchspaces\\f269a46b-ccf7-5d73-abea-4c690281aa53\\build\\local\\windowsZones.xml"): resource busy or locked (EBUSY)", code=-4082)
```

In making this PR I realized I forgot in #740 to actually run
`strip_cldr`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants